Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update build cmd to check tx inputs #3151

Merged
merged 2 commits into from
Sep 2, 2021
Merged

Update build cmd to check tx inputs #3151

merged 2 commits into from
Sep 2, 2021

Conversation

Jimbo4350
Copy link
Contributor

Solves #2996 and #3112

@catch-21 catch-21 linked an issue Sep 2, 2021 that may be closed by this pull request
[FR] - Add input utxo validation to transaction build cmd #2996
Closed
@catch-21
Copy link
Contributor

catch-21 commented Sep 2, 2021

I've noticed that if the tx-in-collateral utxo is missing then only this is listed, even if the tx-in utxos are also missing. Then after fixing collateral input the same error is displayed with the missing tx-in utxos. I kinda get why this would happen, is it possible or even worth making it list them all together?

@catch-21 catch-21 mentioned this pull request Sep 2, 2021
Closed
@catch-21
Copy link
Contributor

catch-21 commented Sep 2, 2021 •
edited
Loading

When using script witness for collateral input the error is:

Command failed: transaction build  Error: Expected key witnessed tx inputs but got script witnessed tx inputs: 
49e462b220ddfe932447c32fd75f47d57b5e4392e37d13b3b8e2b3fe030b4ac5#1

Someone may be reusing the same input and not know which one this refers to. So it might be worth mentioning in the error that this is related to the collateral input to avoid any confusion (unless this error is generic).

@Jimbo4350
Copy link
Contributor Author

I've noticed that if the tx-in-collateral utxo is missing then only this is listed, even if the tx-in utxos are also missing. Then after fixing collateral input the same error is displayed with the missing tx-in utxos. I kinda get why this would happen, is it possible or even worth making it list them all together?

Not worth the effort IMO.

@catch-21 catch-21 mentioned this pull request Sep 2, 2021
Closed
@catch-21 catch-21 linked an issue Sep 2, 2021 that may be closed by this pull request
[FR] - Reject non-key-locked collateral inputs #3112
Closed
@Jimbo4350
Update cli build cmd to check if a tx input exists and to check if the
61464ad 
collateral tx input is a key witnessed tx input.
@Jimbo4350 Jimbo4350 force-pushed the jordan/reject-script-locked-collateral-txins branch from b5d402d to 61464ad Compare September 2, 2021 12:47
@Jimbo4350
Copy link
Contributor Author

bors r+

iohk-bors bot added a commit that referenced this pull request Sep 2, 2021
@iohk-bors @Jimbo4350
Merge #3151
ec65345 
3151: Update build cmd to check tx inputs r=Jimbo4350 a=Jimbo4350

Solves #2996 and #3112

Co-authored-by: Jordan Millar <jordan.millar@iohk.io>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Sep 2, 2021

Timed out.

@Jimbo4350
Copy link
Contributor Author

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Sep 2, 2021

Build succeeded:

@iohk-bors iohk-bors bot merged commit d8f81b9 into master Sep 2, 2021
@iohk-bors iohk-bors bot deleted the jordan/reject-script-locked-collateral-txins branch September 2, 2021 18:58
@newhoggy newhoggy mentioned this pull request Sep 7, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@catch-21 catch-21 catch-21 left review comments

@newhoggy newhoggy newhoggy approved these changes

@DavidEichmann DavidEichmann Awaiting requested review from DavidEichmann

@dcoutts dcoutts Awaiting requested review from dcoutts

@erikd erikd Awaiting requested review from erikd

@JaredCorduan JaredCorduan Awaiting requested review from JaredCorduan

@nc6 nc6 Awaiting requested review from nc6

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FR] - Reject non-key-locked collateral inputs [FR] - Add input utxo validation to transaction build cmd
3 participants
@Jimbo4350 @catch-21 @newhoggy